Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add validate(query:) method to Validatable protocol #2419

Merged
merged 6 commits into from
Jul 16, 2020
Merged

Add validate(query:) method to Validatable protocol #2419

merged 6 commits into from
Jul 16, 2020

Conversation

odanylewycz
Copy link
Contributor

@odanylewycz odanylewycz commented Jul 1, 2020 •
edited by tanner0101

Adds validate(query:) method to Validatable protocol for validating decoded query parameters (#2419).

Previously, you could only validate the content that came from the request body. Now, this addition allows for validating decoded query parameters that conform to the Content protocol.

struct Hello: Content, Validatable {
    name: String?

    static func validations(_ validations: inout Validations) {
        validations.add("name", as: String.self, is: .alphanumeric)
    }
}

And the following URL, http://localhost:8080/model?name=Vapor, you can now validate the query parameter "name" as follows:

try Hello.validate(query: request)
let hello = try req.query.decode(Hello.self)

The existing content validation method validate(_:) has been renamed to validate(content:) to reduce ambiguity.

try Hello.validate(content: request)
let hello = try req.content.decode(Hello.self)

@odanylewycz
Added validate(_ uri: URI) method to Validations struct and Validatab…
91387dd 
…le protocol for validating decoded query parameters.
@0xTim
Copy link
Member

0xTim commented Jul 2, 2020

@odanylewycz nice first-time PR! I've tidied up the comment a bit to use code formatting. Would you mind adding a test or two for the validation stuff?

@odanylewycz
Copy link
Contributor Author

@0xTim Thanks for the suggestion! I forgot to do it in the first place. I have gone ahead and added those tests as you requested. I reverted the prior commit because I misspelled "decoding", it was gonna bother me. Let me know if there is anything else I should add!

0xTim
0xTim approved these changes Jul 6, 2020
View reviewed changes
Copy link
Member

@0xTim 0xTim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tanner0101 tanner0101 added the enhancement New feature or request label Jul 14, 2020
@tanner0101 tanner0101 added this to Awaiting Review in Vapor 4 via automation Jul 14, 2020
tanner0101
tanner0101 reviewed Jul 14, 2020
View reviewed changes
Copy link
Member

@tanner0101 tanner0101 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, thanks! I'm interested for your thoughts about the query label. Otherwise this is good to go.

Sources/Vapor/Validation/Validations.swift Outdated Show resolved Hide resolved
Sources/Vapor/Validation/Validatable.swift Outdated Show resolved Hide resolved
@tanner0101 tanner0101 added the semver-minor Contains new API label Jul 14, 2020
@tanner0101 tanner0101 moved this from Awaiting Review to Awaiting Updates in Vapor 4 Jul 15, 2020
tanner0101
tanner0101 approved these changes Jul 16, 2020
View reviewed changes
Copy link
Member

@tanner0101 tanner0101 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks!

@tanner0101 tanner0101 changed the title Added validate(_ uri: URI) method to Validations struct and Validatable protocol for validating decoded query parameters. Adds validate(query:) method to Validatable protocol Jul 16, 2020
@tanner0101 tanner0101 changed the title Adds validate(query:) method to Validatable protocol Add validate(query:) method to Validatable protocol Jul 16, 2020
@tanner0101 tanner0101 merged commit 69d9c25 into vapor:master Jul 16, 2020
Vapor 4 automation moved this from Awaiting Updates to Done Jul 16, 2020
@tanner0101
Copy link
Member

These changes are now available in 4.22.0

@tanner0101
Copy link
Member

@odanylewycz would you be open to submitting a PR to update https://github.com/vapor/docs/blob/master/4.0/docs/validation.md#validating-request with the new APIs added here?

@odanylewycz
Copy link
Contributor Author

@tanner0101 Certainly, it would be my pleasure! I'll get right on it.

@odanylewycz odanylewycz deleted the query_validations branch July 16, 2020 18:20
@odanylewycz
Copy link
Contributor Author

Made the PR, let me know over on that PR if I need to change anything!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MrLotU MrLotU MrLotU left review comments

@tanner0101 tanner0101 tanner0101 approved these changes

@0xTim 0xTim 0xTim approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request semver-minor Contains new API
Projects
Vapor 4
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@odanylewycz @0xTim @tanner0101 @MrLotU